-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Port bash script naveen521kk to .travis.yml #1
WIP: Port bash script naveen521kk to .travis.yml #1
Conversation
Question: Any necessity for it to be merged in master? Because I used the branch |
Nope, this was definitely my error ! In a bit of a rush now :) |
I've got to be AFK for a while so can't wait to see tests pass, it's possible I've done something silly like bodged the merge with the latest version of your work. |
Currently I am busy I will look into this maybe by tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was some quick questions I came up. Also, can we create a GitHub Action (As it is the only one I know has scheduled build) which checks for upstream for cairo version and other things? A python script would do. Just execute the py script and then create an issue if it not update. Any ideas about this?
.travis.yml
Outdated
name: Build on Python 3.8.3 in Windows | ||
env: PYVER="3.8.3" CAIRO_VERSION=1.17.2 | ||
name: Build on x64 Python 3.8.3 in Windows | ||
env: ARCH=x64 CAIRO_VERSION=1.17.2 PYTHON_PACKAGE=python PYTHON_VERSION="3.8.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be updated to 3.8.5
as it is released now.
env: ARCH=x64 CAIRO_VERSION=1.17.2 PYTHON_PACKAGE=python PYTHON_VERSION="3.8.3" | |
env: ARCH=x64 CAIRO_VERSION=1.17.2 PYTHON_PACKAGE=python PYTHON_VERSION="3.8.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot...
We can automate this, I spoke to dependencies.io who gave me an account for open source stuff + that can generate diffs and PRs for this sort of thing.
It's annoying nuget doesn't accept a query like 3.7.x and give the highest match, I wonder if chocolatey can do it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use a query like 3.7
I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I just found that choco and nuget are one and the same thing in structure. Choco uses nugets structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chocolatey has some extensions, I'm inclined to switch back to it, as it has some advantages.
Native support in Travis: one less thing to install, and Travis' cache support seems to include it.
You can force it to install for 32 or 64 bit as needed.
Semver: not supported now, but they have a ticket.
chocolatey/choco#1610
In the meantime I think we might be able to do semver support with a combination of powershell and the output of choco search
or hit their API directly with cURL
.
This would be good as we could specify the python version with 2 digit versions, not 3, so one less thing to support and patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can pass - -x86 to install 32 bit versions, I haven't tried it yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't help much. Nuget python is build for CI ,but choco is using default py installer which may be troublesome. So better to use nuget for x86.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, will stay on nuget, if there are questions around chocolatey.
Do you know what the issues with the default installer are?
I haven't used windows as my default OS for a while, well aware windows + python + C extensions can get pretty painful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda use windows as my default one. Maybe because I don't know C or something. I just know quite a few basic of python and got to know about this because of manim. Some video tool which use pycairo as it's base. I don't even have C compilers in my PC and that made me do this. But yeah Nuget x86 py build should be used or we should use something kinada docker or something but not required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be easily fixed if we were to use GitHub Actions.
What do you think about this? @stuaxo |
I can def do the github action to generate patch PRs, I have one I need to push upstream to cairo-windows that already does this, so fairly straightforward. I'm a bit new to github actions so wasn't sure the best events for it to run on, so am thinking scheduled daily, and also available from a webhook (I imagine only @lazka will have the token for that). As soon as someone merges the PR then I guess that will trigger a build and release (on github at least). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggetsions
About test failing I remember I read somewhere where lazka said about We can skip that test for windows see https://docs.pytest.org/en/latest/skipping.html#id1 |
75df715
to
895a850
Compare
Fix adding python to PATH Co-authored-by: Naveen M K <[email protected]>
Install wheel for all OSs, - via @naveen521kk Co-authored-by: Naveen M K <[email protected]>
Definitely doable, I think I wrote somewhere else we can do with dependencies.io and their deps tool. I wonder which things we want to check on... if we can get rid of the need for an exact python build. then that leaves |
707b4bd
to
a18151a
Compare
How about py |
We're OK until October the 5th for 3.9 |
I will need to check and work something and will do in two days and later it can be merged. |
Now tests should be passing. |
@stuaxo We messed up something with x86 builds. |
@stuaxo If the tests pass this is ready to be merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can change the test to check for minimum version to also check for Windows...
Do we know everything works OK with TeeSurface not enabled?
We might be able to just enable TeeSurface upstream, I don't have the dev environment setup to try this right now, but if it works we could always patch upstream and this will be fixed for good (it seems like msys has this enabled).
I noticed about that. Maybe let's use that? Include the |
I am such a noob here. I don't know C lol. So, I have no much experience in pycairo also. |
I got some weird ideas. @stuaxo
|
Looks like I won't be able to spend much time on this in the upcoming week and I plan to take a long break. So I would be working on this PR next week. |
I've got a similar time problem.. Will reply to this shortly though :) |
Yeah, definitely if @lazka is up for it.
This makes sense, but I reckon we can probably be able to land this first, as it does seem to work. We should verify there are no new issues using this when pygobject is already installed, then it should be good to go. I'm also going to have less time, but should be able to help land the WHL support at least. |
Question: Does the whl available on https://www.lfd.uci.edu/~gohlke/pythonlibs/#pycairo enables tee surface? We need to look into it. |
Yes it works there. |
@stuaxo If you could https://github.com/naveen521kk/pycairo/runs/1035437105?check_suite_focus=true#step:6:165 help me fix this I would be very happy and also it can be used by preshing/cairo-windows and use automatic releasing using Github actions. After that, maybe lets depend on then. |
I'll have a look, I'm back @ work at the moment so won't be until later. I've been looking at getting TeeSurface enabled on cairo-windows, which would mean we don't nee to change any tests @preshing has suggested a solution to get it working, but I won't have time until @ least later on, I started a PR there to generate make PRs when there are new releases of cairo, freetype etc update using dependencies.io |
Quick feedback: I can see it's building with VS2019, I don't really know the differences between 2017 + 2019, but @preshing told me that cairo-windows is currently built on 2017, not 19, so there may be some compatibility issue. |
Yes @stuaxo does https://github.com/naveen521kk/pycairo/runs/1046803166?check_suite_focus=true work? I am asking on tee surface did that work? If you any test environment download the wheels from artifact's and test it once locally. It that works we can tell @preshing to release it so. And yes tee surface problem will be over. |
@naveen521kk I'm away from my computer, any idea why the x64 bit build fails? All I tried was telling Travis to rebuild from my phone, think I need to verify from windows VM when in front of computer later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is what happens.
.travis.yml
Outdated
- if [ "$TRAVIS_OS_NAME" == "windows" ]; then $TMP/nuget install python -Version $PYTHON_VERSION -OutputDirectory $TMP/nuget-$ARCH; fi | ||
- if [ "$TRAVIS_OS_NAME" == "windows" ]; then export INCLUDE="$TMP/cairo-windows-$CAIRO_VERSION/include"; fi | ||
- if [ "$TRAVIS_OS_NAME" == "windows" ]; then export LIB=$TMP/cairo-windows-$CAIRO_VERSION/lib/$ARCH; fi | ||
- if [ "$TRAVIS_OS_NAME" == "windows" ]; then export PATH=$TMP/nuget-$ARCH/python.$PYTHON_VERSION/tools/:$TMP/nuget-$ARCH/python.$PYTHON_VERSION/tools/Scripts:$PATH; fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found why it fails. Because the nuget package pythonx86
contains x86 build of python and it does pull x64 python and then later tries to build for x86 which makes is fail. :( Check my shell script.
Update cairo-windows version to "with-tee" tag.
Revert cairo-windows version, need to do this in front of computer with proper time :)
Broke version numbers as trying to type with 2 mins to spare :)
Fix cairo-windows URL
I need to stop trying to edit this at the same time as trying to get a 3 year old ready to get out of the house :) I'll come back to this when I can give it my full attention and stop adding duff commits :) Don't worry, I will squash the commits before we push anything to the pycairo proper :) |
There's a pretty good chance it might be the when the work week starts (Tuesday it's a long weekend) when I can look at stuff properly, as it's almost all small person time, then just tiny bits of adult time during the weekend (and some small bits of coding time)! |
Maybe I will work if I get some time. |
Specified the variable `$PYTHON_PACKAGE` to install. cc. @stuaxo
Any idea why https://travis-ci.com/github/naveen521kk/pycairo/jobs/379364608 fail? @stuaxo |
Squashed and merged to pygobject/pycairo. |
Port WHL build script @naveen521kk to plain
.travis.yml
.travis.yml
Minor changes:
TODO
Having added the newer tests, test_typing fails.